lib/repo: Search a list of paths in gpgkeypath for gpg keys
authorrfairley <rfairley@redhat.com>
Tue, 6 Nov 2018 20:25:15 +0000 (15:25 -0500)
committerAtomic Bot <atomic-devel@projectatomic.io>
Wed, 21 Nov 2018 17:03:10 +0000 (17:03 +0000)
This allows specifying gpgpath as list of
paths that can point to a file or a directory. If a directory path
is given, paths to all regular files in the directory are added
to the remote as gpg ascii keys. If the path is not a directory,
the file is directly added (whether regular file, empty - errors
will be reported later when verifying gpg keys e.g. when pulling).

Adding the gpgkeypath property looks like:

ostree --repo=repo remote add --set=gpgpath="/path/key1.asc,/path/keys.d" R1 https://example.com/some/remote/ostree/repo

Closes #773

Closes: #1773
Approved by: cgwalters

man/ostree.xml
src/libostree/ostree-gpg-verifier.c
src/libostree/ostree-gpg-verifier.h
src/libostree/ostree-repo.c
src/libotutil/ot-keyfile-utils.c
src/libotutil/ot-keyfile-utils.h
tests/test-remote-gpg-import.sh

index d1c156659ed8fe1f409f7b4e273a339138f21c80..f865982e7f6bb034406de2b599ce5b4385c98981 100644 (file)
@@ -432,6 +432,7 @@ Boston, MA 02111-1307, USA.
 
     <refsect1>
         <title>Examples</title>
+
         <para>
             For specific examples, please see the man page regarding the specific ostree command.  For example:
         </para>
@@ -445,28 +446,32 @@ Boston, MA 02111-1307, USA.
 
         <para>
           OSTree supports signing commits with GPG.  Operations on the system
-         repository by default use keyring files in
+          repository by default use keyring files in
           <filename>/usr/share/ostree/trusted.gpg.d</filename>.  Any
           public key in a keyring file in that directory will be
           trusted by the client.  No private keys should be present
           in this directory.
         </para>
         <para>
-         In addition to the system repository, OSTree supports two
-         other paths.  First, there is a
-         <literal>gpgkeypath</literal> option for remotes, which must
-         point to the filename of an ASCII-armored key.
-       </para>
-       <para>Second, there is support for a per-remote
-           <filename><replaceable>remotename</replaceable>.trustedkeys.gpg</filename>
-           file stored in the toplevel of the repository (alongside
-           <filename>objects/</filename> and such). This is
-           particularly useful when downloading content that may not
-           be fully trusted (e.g. you want to inspect it but not
-           deploy it as an OS), or use it for containers.  This file
-           is written via <command>ostree remote add
-           --gpg-import</command>.
-       </para>
+          In addition to the system repository, OSTree supports two
+          other paths.  First, there is a
+          <literal>gpgkeypath</literal> option for remotes, which must point
+          to the filename of an ASCII-armored GPG key, or a directory containing
+          ASCII-armored GPG keys to import.  Multiple file and directory paths
+          to import from can be specified, as a comma-separated list of paths.  This option
+          may be specified by using <command>--set</command> in <command>ostree remote add</command>.
+        </para>
+        <para>
+          Second, there is support for a per-remote
+          <filename><replaceable>remotename</replaceable>.trustedkeys.gpg</filename>
+          file stored in the toplevel of the repository (alongside
+          <filename>objects/</filename> and such).  This is
+          particularly useful when downloading content that may not
+          be fully trusted (e.g. you want to inspect it but not
+          deploy it as an OS), or use it for containers.  This file
+          is written via <command>ostree remote add
+          --gpg-import</command>.
+        </para>
     </refsect1>
 
     <refsect1>
index 1b70f8aa4eea54f36ab2c5f2777a7a47cd84f413..a279348eb395bcbe563a9842c6c3db52d492120c 100644 (file)
@@ -304,12 +304,82 @@ _ostree_gpg_verifier_add_key_ascii_file (OstreeGpgVerifier *self,
   g_ptr_array_add (self->key_ascii_files, g_strdup (path));
 }
 
+gboolean
+_ostree_gpg_verifier_add_keyfile_path (OstreeGpgVerifier   *self,
+                                       const char          *path,
+                                       GCancellable        *cancellable,
+                                       GError             **error)
+{
+  g_autoptr(GError) temp_error = NULL;
+  if (!_ostree_gpg_verifier_add_keyfile_dir_at (self, AT_FDCWD, path,
+                                                cancellable, &temp_error))
+    {
+      g_assert (temp_error);
+
+      /* If failed due to not being a directory, add the file as an ascii key. */
+      if (g_error_matches (temp_error, G_IO_ERROR, G_IO_ERROR_NOT_DIRECTORY))
+        {
+          g_clear_error (&temp_error);
+
+          _ostree_gpg_verifier_add_key_ascii_file (self, path);
+        }
+      else
+        {
+          g_propagate_error (error, g_steal_pointer (&temp_error));
+
+          return FALSE;
+        }
+    }
+  return TRUE;
+}
+
+/* Add files that exist one level below the directory at @path as ascii
+ * key files. If @path cannot be opened as a directory,
+ * an error is returned.
+ */
+gboolean
+_ostree_gpg_verifier_add_keyfile_dir_at (OstreeGpgVerifier   *self,
+                                         int                  dfd,
+                                         const char          *path,
+                                         GCancellable        *cancellable,
+                                         GError             **error)
+{
+  g_auto(GLnxDirFdIterator) dfd_iter = { 0, };
+
+  if (!glnx_dirfd_iterator_init_at (dfd, path, FALSE,
+                                    &dfd_iter, error))
+    return FALSE;
+
+  g_debug ("Adding GPG keyfile dir %s to verifier", path);
+
+  while (TRUE)
+    {
+      struct dirent *dent;
+
+      if (!glnx_dirfd_iterator_next_dent_ensure_dtype (&dfd_iter, &dent,
+                                                       cancellable, error))
+        return FALSE;
+      if (dent == NULL)
+        break;
+
+      if (dent->d_type != DT_REG)
+        continue;
+
+      /* TODO: Potentially open the files here and have the GPG verifier iterate
+      over the fds. See https://github.com/ostreedev/ostree/pull/1773#discussion_r235421900. */
+      g_autofree char *iter_path = g_build_filename (path, dent->d_name, NULL);
+
+      _ostree_gpg_verifier_add_key_ascii_file (self, iter_path);
+    }
+
+  return TRUE;
+}
+
 gboolean
 _ostree_gpg_verifier_add_keyring_dir (OstreeGpgVerifier   *self,
                                       GFile               *path,
                                       GCancellable        *cancellable,
                                       GError             **error)
-
 {
   return _ostree_gpg_verifier_add_keyring_dir_at (self, AT_FDCWD,
                                                   gs_file_get_path_cached (path),
@@ -322,7 +392,6 @@ _ostree_gpg_verifier_add_keyring_dir_at (OstreeGpgVerifier   *self,
                                          const char          *path,
                                          GCancellable        *cancellable,
                                          GError             **error)
-
 {
   g_auto(GLnxDirFdIterator) dfd_iter = { 0, };
   if (!glnx_dirfd_iterator_init_at (dfd, path, FALSE,
index 4fe62d98e7ff90fc5e8dd464b23eab197beb8052..634d33b29971afcd3e519d387dde49f598989f86 100644 (file)
@@ -75,4 +75,17 @@ void _ostree_gpg_verifier_add_keyring_file (OstreeGpgVerifier *self,
 void _ostree_gpg_verifier_add_key_ascii_file (OstreeGpgVerifier *self,
                                               const char        *path);
 
+gboolean
+_ostree_gpg_verifier_add_keyfile_path (OstreeGpgVerifier   *self,
+                                       const char          *path,
+                                       GCancellable        *cancellable,
+                                       GError             **error);
+
+gboolean
+_ostree_gpg_verifier_add_keyfile_dir_at (OstreeGpgVerifier   *self,
+                                         int                  dfd,
+                                         const char          *path,
+                                         GCancellable        *cancellable,
+                                         GError             **error);
+
 G_END_DECLS
index fa3c2a940f9dcbc1bbbaafc260c05d2a49a32b82..f5231db98908a5740826b0f88a75ceee95c8a318 100644 (file)
@@ -5081,7 +5081,6 @@ _ostree_repo_gpg_verify_data_internal (OstreeRepo    *self,
     }
   else if (remote_name != NULL)
     {
-      g_autofree char *gpgkeypath = NULL;
       /* Add the remote's keyring file if it exists. */
 
       g_autoptr(OstreeRemote) remote = NULL;
@@ -5100,12 +5099,19 @@ _ostree_repo_gpg_verify_data_internal (OstreeRepo    *self,
           add_global_keyring_dir = FALSE;
         }
 
-      if (!ot_keyfile_get_value_with_default (remote->options, remote->group, "gpgkeypath", NULL,
-                                              &gpgkeypath, error))
+      g_auto(GStrv) gpgkeypath_list = NULL;
+
+      if (!ot_keyfile_get_string_as_list (remote->options, remote->group, "gpgkeypath",
+                                          ";,", &gpgkeypath_list, error))
         return NULL;
 
-      if (gpgkeypath)
-        _ostree_gpg_verifier_add_key_ascii_file (verifier, gpgkeypath);
+      if (gpgkeypath_list)
+        {
+          for (char **iter = gpgkeypath_list; *iter != NULL; ++iter)
+            if (!_ostree_gpg_verifier_add_keyfile_path (verifier, *iter,
+                                                        cancellable, error))
+              return NULL;
+        }
     }
 
   if (add_global_keyring_dir)
index 3b29377f2f1ca7c0444ba5582e4524cf84ff5223..a0ab75cc0cf5b60fefa0fe37197e31a4ed7527b5 100644 (file)
@@ -101,6 +101,107 @@ ot_keyfile_get_value_with_default (GKeyFile      *keyfile,
   return ret;
 }
 
+/* Read the value of key as a string. If the value string contains
+ * one of the separators and none of the others, read the
+ * string as a NULL-terminated array out_value. If the value string contains
+ * none of the separators, read the string as a single entry into a
+ * NULL-terminated array out_value. If the value string contains multiple of
+ * the separators, an error is given.
+ * Returns TRUE on success, FALSE on error. */
+gboolean
+ot_keyfile_get_string_as_list (GKeyFile      *keyfile,
+                               const char    *section,
+                               const char    *key,
+                               const char    *separators,
+                               char        ***out_value,
+                               GError       **error)
+{
+  guint sep_count = 0;
+  gchar sep = '\0';
+  g_autofree char  *value_str = NULL;
+  g_autofree char **value_list = NULL;
+
+  g_return_val_if_fail (keyfile != NULL, FALSE);
+  g_return_val_if_fail (section != NULL, FALSE);
+  g_return_val_if_fail (key != NULL, FALSE);
+  g_return_val_if_fail (separators != NULL, FALSE);
+
+  if (!ot_keyfile_get_value_with_default (keyfile, section, key, NULL,
+                                          &value_str, error))
+    return FALSE;
+
+  if (value_str)
+    {
+      for (size_t i = 0; i < strlen (separators) && sep_count <= 1; i++)
+        {
+          if (strchr (value_str, separators[i]))
+            {
+              sep_count++;
+              sep = separators[i];
+            }
+        }
+
+      if (sep_count == 0)
+        {
+          value_list = g_new (gchar *, 2);
+          value_list[0] = g_steal_pointer (&value_str);
+          value_list[1] = NULL;
+        }
+      else if (sep_count == 1)
+        {
+          if (!ot_keyfile_get_string_list_with_default (keyfile, section, key,
+                                                        sep, NULL, &value_list, error))
+            return FALSE;
+        }
+      else
+        {
+          return glnx_throw (error, "key value list contains more than one separator");
+        }
+    }
+
+  ot_transfer_out_value (out_value, &value_list);
+  return TRUE;
+}
+
+gboolean
+ot_keyfile_get_string_list_with_default (GKeyFile      *keyfile,
+                                         const char    *section,
+                                         const char    *key,
+                                         char           separator,
+                                         char         **default_value,
+                                         char        ***out_value,
+                                         GError       **error)
+{
+  g_autoptr(GError) temp_error = NULL;
+
+  g_return_val_if_fail (keyfile != NULL, FALSE);
+  g_return_val_if_fail (section != NULL, FALSE);
+  g_return_val_if_fail (key != NULL, FALSE);
+
+  g_key_file_set_list_separator (keyfile, separator);
+
+  g_autofree char **ret_value = g_key_file_get_string_list (keyfile, section,
+                                                            key, NULL, &temp_error);
+
+  if (temp_error)
+    {
+      if (g_error_matches (temp_error, G_KEY_FILE_ERROR,
+                           G_KEY_FILE_ERROR_KEY_NOT_FOUND))
+        {
+          g_clear_error (&temp_error);
+          ret_value = default_value;
+        }
+      else
+        {
+          g_propagate_error (error, g_steal_pointer (&temp_error));
+          return FALSE;
+        }
+    }
+
+  ot_transfer_out_value (out_value, &ret_value);
+  return TRUE;
+}
+
 gboolean
 ot_keyfile_copy_group (GKeyFile   *source_keyfile,
                        GKeyFile   *target_keyfile,
index e136ee47a6c8295b1a2e8983b7e4aa127594113f..2dcb899cf7670e8e82e0a0fb3e9c206e69696c79 100644 (file)
@@ -44,6 +44,23 @@ ot_keyfile_get_value_with_default (GKeyFile      *keyfile,
                                    char         **out_value,
                                    GError       **error);
 
+gboolean
+ot_keyfile_get_string_as_list (GKeyFile      *keyfile,
+                               const char    *section,
+                               const char    *key,
+                               const char    *separators,
+                               char        ***out_value_list,
+                               GError       **error);
+
+gboolean
+ot_keyfile_get_string_list_with_default (GKeyFile      *keyfile,
+                                         const char    *section,
+                                         const char    *key,
+                                         char           separator,
+                                         char         **default_value,
+                                         char        ***out_value,
+                                         GError       **error);
+
 gboolean
 ot_keyfile_copy_group (GKeyFile   *source_keyfile,
                        GKeyFile   *target_keyfile,
index f816429c4d06802384c13653f39567eb0d26c805..1bb42d82e8ca7fbc89322cfe99104d171d6251a6 100755 (executable)
@@ -154,6 +154,79 @@ ${OSTREE} prune --refs-only
 ${OSTREE} remote add --set=gpgkeypath=${test_tmpdir}/gpghome/key3.asc R4 $(cat httpd-address)/ostree/gnomerepo
 ${OSTREE} pull R4:main >/dev/null
 
+# Test gpgkeypath success with multiple keys to try
+${OSTREE} remote add --set=gpgkeypath=${test_tmpdir}/gpghome/key1.asc,${test_tmpdir}/gpghome/key2.asc,${test_tmpdir}/gpghome/key3.asc R7 $(cat httpd-address)/ostree/gnomerepo
+${OSTREE} pull R7:main >/dev/null
+
+# Test gpgkeypath failure with multiple keys but none in keyring
+${OSTREE} remote add --set=gpgkeypath=${test_tmpdir}/gpghome/key1.asc,${test_tmpdir}/gpghome/key2.asc R8 $(cat httpd-address)/ostree/gnomerepo
+if ${OSTREE} pull R8:main 2>err.txt; then
+    assert_not_reached "Unexpectedly succeeded at pulling with different key"
+fi
+assert_file_has_content err.txt "GPG signatures found, but none are in trusted keyring"
+
+# Test gpgkeypath success with directory containing a valid key
+${OSTREE} remote add --set=gpgkeypath=${test_tmpdir}/gpghome/ R9 $(cat httpd-address)/ostree/gnomerepo
+${OSTREE} pull R9:main >/dev/null
+
+# Test gpgkeypath failure with nonexistent directory
+${OSTREE} remote add --set=gpgkeypath=${test_tmpdir}/gpghome/INVALIDKEYDIRPATH/ R10 $(cat httpd-address)/ostree/gnomerepo
+if ${OSTREE} pull R10:main 2>err.txt; then
+    assert_not_reached "Unexpectedly succeeded at pulling with nonexistent key directory"
+fi
+assert_file_has_content err.txt "INVALIDKEYDIRPATH.*No such file or directory"
+
+# Test gpgkeypath failure with a directory containing a valid key, and a nonexistent key
+${OSTREE} remote add --set=gpgkeypath=${test_tmpdir}/gpghome/,${test_tmpdir}/gpghome/INVALIDKEYPATH.asc R11 $(cat httpd-address)/ostree/gnomerepo
+if ${OSTREE} pull R11:main 2>err.txt; then
+    assert_not_reached "Unexpectedly succeeded at pulling with nonexistent key"
+fi
+assert_file_has_content err.txt "INVALIDKEYPATH.*No such file or directory"
+
+# Test gpgkeypath success with a directory containing a valid key, and a key not in keyring
+${OSTREE} remote add --set=gpgkeypath=${test_tmpdir}/gpghome/,${test_tmpdir}/gpghome/key1.asc R12 $(cat httpd-address)/ostree/gnomerepo
+${OSTREE} pull R12:main >/dev/null
+
+# Test gpgkeypath failure with a nonexistent directory, and a valid key
+${OSTREE} remote add --set=gpgkeypath=${test_tmpdir}/gpghome/INVALIDKEYDIRPATH/,${test_tmpdir}/gpghome/key3.asc R13 $(cat httpd-address)/ostree/gnomerepo
+if ${OSTREE} pull R13:main 2>err.txt; then
+    assert_not_reached "Unexpectedly succeeded at pulling with nonexistent key directory"
+fi
+assert_file_has_content err.txt "INVALIDKEYDIRPATH.*No such file or directory"
+
+# Test gpgkeypath failure with a nonexistent directory and a nonexistent key
+${OSTREE} remote add --set=gpgkeypath=${test_tmpdir}/gpghome/INVALIDKEYDIRPATH/,${test_tmpdir}/gpghome/INVALIDKEYPATH.asc R14 $(cat httpd-address)/ostree/gnomerepo
+if ${OSTREE} pull R14:main 2>err.txt; then
+    assert_not_reached "Unexpectedly succeeded at pulling with nonexistent key"
+fi
+assert_file_has_content err.txt "INVALIDKEYDIRPATH.*No such file or directory"
+
+# Test gpgkeypath success for no trailing slash in directory path
+${OSTREE} remote add --set=gpgkeypath=${test_tmpdir}/gpghome R15 $(cat httpd-address)/ostree/gnomerepo
+${OSTREE} pull R15:main >/dev/null
+
+# Test gpgkeypath failure with prefixed separator giving an empty path, and a nonexistent key
+${OSTREE} remote add --set=gpgkeypath=,${test_tmpdir}/gpghome/INVALIDKEYPATH.asc R16 $(cat httpd-address)/ostree/gnomerepo
+if ${OSTREE} pull R16:main 2>err.txt; then
+    assert_not_reached "Unexpectedly succeeded at pulling with nonexistent key"
+fi
+assert_file_has_content err.txt "().*No such file or directory"
+
+# Test gpgkeypath success with suffixed separator
+${OSTREE} remote add --set=gpgkeypath=${test_tmpdir}/gpghome/key3.asc, R17 $(cat httpd-address)/ostree/gnomerepo
+${OSTREE} pull R17:main >/dev/null
+
+# Test gpgkeypath success with multiple keys specified, with semicolons
+${OSTREE} remote add --set=gpgkeypath="${test_tmpdir}/gpghome/key1.asc;${test_tmpdir}/gpghome/key2.asc;${test_tmpdir}/gpghome/key3.asc" R18 $(cat httpd-address)/ostree/gnomerepo
+${OSTREE} pull R18:main >/dev/null
+
+# Test gpgkeypath failure multiple keys specified, with mix of commas and semicolons
+${OSTREE} remote add --set=gpgkeypath="${test_tmpdir}/gpghome/key1.asc,${test_tmpdir}/gpghome/key2.asc;${test_tmpdir}/gpghome/key3.asc" R19 $(cat httpd-address)/ostree/gnomerepo
+if ${OSTREE} pull R19:main 2>err.txt; then
+    assert_not_reached "Unexpectedly succeeded at pulling with invalid gpgkeypath value"
+fi
+assert_file_has_content err.txt ".*key value list contains more than one separator"
+
 rm repo/refs/remotes/* -rf
 ${OSTREE} prune --refs-only